Skip to content

chore: configure retries in pre-upgrade hook job (helm)#2710

Merged
sozercan merged 2 commits into
open-policy-agent:masterfrom
aramase:aramase/i/2685
Apr 28, 2023
Merged

chore: configure retries in pre-upgrade hook job (helm)#2710
sozercan merged 2 commits into
open-policy-agent:masterfrom
aramase:aramase/i/2685

Conversation

@aramase
Copy link
Copy Markdown
Contributor

@aramase aramase commented Apr 19, 2023

fixes #2685

Signed-off-by: Anish Ramasekar <anish.ramasekar@gmail.com>
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

Patch coverage has no change and project coverage change: +0.20 🎉

Comparison is base (c994fb9) 52.64% compared to head (a2ae27e) 52.84%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2710      +/-   ##
==========================================
+ Coverage   52.64%   52.84%   +0.20%     
==========================================
  Files         123      123              
  Lines       10941    10941              
==========================================
+ Hits         5760     5782      +22     
+ Misses       4721     4705      -16     
+ Partials      460      454       -6     
Flag Coverage Δ
unittests 52.84% <ø> (+0.20%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 3 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Copy Markdown
Member

@sozercan sozercan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! LGTM

helm.sh/hook-delete-policy: "hook-succeeded,before-hook-creation"
spec:
backoffLimit: 0
backoffLimit: 3
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not take the default? something less to configure?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default is 6 and retries are with exponential backoff (I've mentioned this in the PR description as well) which could cause the job to hang for longer in case of actual failures. The goal with configuring it to 3 was so there are some retries but still we could fail fast. Let me know what you think!

@sozercan sozercan requested a review from maxsmythe April 27, 2023 01:33
Copy link
Copy Markdown
Contributor

@maxsmythe maxsmythe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@sozercan sozercan merged commit 124c600 into open-policy-agent:master Apr 28, 2023
@aramase aramase deleted the aramase/i/2685 branch April 28, 2023 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

change backOffLimit in pre-upgrade hook jobs to allow retries

5 participants